Resilient saved object migration algorithm #78413

rudolf · 2020-09-24T12:27:13Z

Summary

Implements: #75780

Current status

Having tried to clone indices we ran into several versions of Kibana which have incompatible mappings making cloning impossible.
I tried to just do a reindex operation instead of cloning, but this had a risk of lost deletes
I tried to use an alias and write block to create my own 'reindex block', this works most of the time, but there was a risk that the node that started the reindex is no longer part of the cluster, but that the node is still sending reindex batches which could lead to lost deletes
Finally we're reindexing into a temp index, then cloning that index into a target index. Deletes only happen on the cloned target index so that prevents any lost deletes. We delete the temp index after completing the migration. This means that it's possible for us to do two full reindex operations when one instance doesn't yet see that the migration is complete, but the temp index gets deleted before it initiates a reindex. This adds unnecessary load but otherwise doesn't have an impact.
I've done the following manual testing:
- Test migrations against a data set of 100k documents, performance is similar
- Tested with a 6.8.13 data set (the integration tests include datasets for 7.3, 7.7 and 8.0)
- Tested that all actions are idempotent by repeating the following process:
  1. Start Kibana
  2. Fail at step N
  3. Start Kibana
  4. Fail at step N
  5. N++; repeat

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

src/core/server/saved_objects/migrationsv2/index.ts

mshustov · 2020-09-28T15:41:33Z

src/core/server/saved_objects/migrationsv2/index.ts

+  };
+  const control = state.controlState;
+  if (control === 'INIT') {
+    return delay(Actions.fetchAliases(client, ['.kibana', '.kibana_current', '.kibana_7.11.0']));


nit: the action is tightly coupled with the state. Is it possible to collocate them closer?

src/core/server/saved_objects/migrationsv2/index.ts

joshdover

Overall, I'm very excited about this FSM-based approach. This will make it much easier for us to test very specific states in isolation. I took a very similar approach with the reindexing mechanism in the Upgrade Assistant and it was much easier to reason about.

If we add detailed debug logging for the state & actions, understanding failures in production will also be much eaiser for our support team. We should definitely add detailed documentation on what these states mean and how to diagnose problems (but make clear that these states are not 'stable' so that we can change them between versions, if necessary)

src/core/server/saved_objects/migrationsv2/index.ts

kobelb · 2020-11-30T18:06:55Z

The new saved-object migrations algorithm relies on the fact that the migrations defined by the plugins are deterministic. Do we have any controls in place to ensure that all current and future migrations are deterministic?

… one request

pgayvallet

Overall, the code coverage is way better now.

Still got some nits/comments, but nothing that significant.

pgayvallet · 2020-12-14T12:58:19Z

src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.ts

+    if (log.type === 'transition') {
+      logger.info(logMessagePrefix + `${log.prevControlState} -> ${log.controlState}`, log.state);
+    }


NIT: logger.info(${logMessagePrefix} ${log.prevControlState} -> ${log.controlState}, log.state); (multiple occurences)

src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.ts

pgayvallet · 2020-12-14T13:26:23Z

src/core/server/saved_objects/migrationsv2/state_action_machine.test.ts

+    });
+
+  const countUntilThree = countUntilModel(3);
+  const finalStateP = stateActionMachine(state, next, countUntilThree);


NIT: this can throw. I would at least move it to a beforeAll or beforeEach block

pgayvallet · 2020-12-14T13:29:06Z

src/core/server/saved_objects/migrationsv2/next.ts

+      pipe(
+        TaskEither.tryCatch(
+          () => transformRawDocs(state.outdatedDocuments),
+          (e) => {
+            throw e;
+          }
+        ),
+        TaskEither.chain((docs) =>
+          Actions.bulkOverwriteTransformedDocuments(client, state.targetIndex, docs)
+        )
+      ),


NIT: should probably be 'extracted' to the action creator

pgayvallet · 2020-12-14T13:34:10Z

src/core/server/saved_objects/migrationsv2/next.ts

+    if (state.controlState === 'DONE' || state.controlState === 'FATAL') {
+      // Return null if we're in one of the terminating states
+      return null;


ultraNIT: could introduce either

isTerminal(controlState)

terminalStates array + terminalStates.includes(state.controlState)

src/core/server/saved_objects/migrations/kibana/kibana_migrator.ts

src/core/server/saved_objects/migrationsv2/actions/catch_retryable_es_client_errors.test.ts

src/core/server/saved_objects/migrationsv2/model.ts

pgayvallet · 2020-12-14T20:52:34Z

src/core/server/saved_objects/migrationsv2/model.ts

+  const outdatedDocumentsQuery = {
+    bool: {
+      should: Object.entries(migrationVersionPerType).map(([type, latestVersion]) => ({
+        bool: {
+          must: { term: { type } },
+          must_not: { term: { [`migrationVersion.${type}`]: latestVersion } },
+        },
+      })),
+    },
+  };


This feels a little strange that this is put in the state, but OTOH not doing it would force to put migrationVersionPerType in it instead, so not sure which one is the best

yeah @restrry had a good idea to create a KibanaModel that could contain all the state and logic that's static, things we don't collect as part of the algorithm but rather that's injected into it at the start. We could still attach the KibanaModel onto the state, but it could for instance be a class and wouldn't have to be a POJO like the rest of the state.

Didn't get around to it though :(

pgayvallet · 2020-12-14T21:19:05Z

src/core/server/saved_objects/migrationsv2/model.ts

+  if (stateP.controlState === 'INIT') {
+    const res = resW as ExcludeRetryableEsError<ResponseType<typeof stateP.controlState>>;
+
+    if (Either.isRight(res)) {
+      const indices = res.right;
+      const aliases = getAliases(indices);
+
+      if (


The file would still need to be split into per-state files.

…rocess

rudolf · 2020-12-15T17:31:32Z

@elasticmachine merge upstream

rudolf · 2020-12-15T20:37:48Z

I'm merging this so that we can get a snapshot build for QA to test with and so that we can backport it to 7.x (7.12) which will make it a lot simpler to test with 6.x data.

kibanamachine · 2020-12-17T20:44:39Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kibanamachine · 2020-12-18T18:50:10Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 4a7f2f7

Metrics [docs]

Distributable file count

id	before	after	diff
`default`	47241	48013	+772
`oss`	27540	27859	+319

History

💚 Build #94604 succeeded 4a7f2f7
💚 Build #94401 succeeded e815df4
💔 Build #94396 failed 11dc7cf
💔 Build #94363 failed eea4f9e
💔 Build #94333 failed 286bdfa

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kibanamachine · 2020-12-18T20:54:38Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

* Resilient saved object migration algorithm (#78413) * Initial structure of migration state-action machine * Fix type import * Retries with exponential back off * Use discriminated union for state type * Either type for actions * Test exponential retries * TaskEither types for actions * Fetch indices instead of aliases so we can collect all index state in one request * Log document id if transform fails * WIP: Legacy pre-migrations * UPDATE_TARGET_MAPPINGS * WIP OUTDATED_DOCUMENTS_TRANSFORM * Narrow res types depending on control state * OUTDATED_DOCUMENTS_TRANSFORM * Use .kibana instead of .kibana_current * rename control states TARGET_DOCUMENTS* -> OUTDATED_DOCUMENTS* * WIP MARK_VERSION_INDEX_READY * Fix and expand INIT -> * transition tests * Add alias/index name helper functions * Add feature flag for enabling v2 migrations * split state_action_machine, reindex legacy indices * Don't use a scroll search for migrating outdated documents * model: test control state progressions * Action integration tests * Fix existing tests and type errors * snapshot_in_progress_exception can only happen when closing/deleting an index * Retry steps up to 10 times * Update api.md documentation files * Further actions integration tests * Action unit tests * Fix actions integration tests * Rename actions to be more domain-specific * Apply suggestions from code review Co-authored-by: Josh Dover <me@joshdover.com> * Review feedback: polish and flesh out inline comments * Fix unhandled rejections in actions unit tests * model: only delay retryable_es_client_error, reset for other left responses * Actions unit tests * More inline comments * Actions: Group index settings under 'index' key * bulkIndex -> bulkOverwriteTransformedDocuments to be more domain specific * state_action_machine tests, fix and add additional tests * Action integration tests: updateAndPickupMappings, searchForOutdatedDocuments * oops: uncomment commented out code * actions integration tests: rejection for createIndex * update state properties: clearer names, mark all as readonly * add state properties currentAlias, versionAlias, legacyIndex and test for invalid version scheme in index names * Use CONSTANTS for constants :D * Actions: Clarify behaviour and impact of acknowledged: false responses * Use consistent vocabulary for action responses * KibanaMigrator test for migrationsV2 * KibanaMigrator test for FATAL state and action exceptions in v2 migrations * Fix ts error in test * Refactor: split index file up into a file per model, next, types * next: use partial application so we don't generate a nextActionMap on every call * move logic from index.ts to migrations_state_action_machine.ts and test * add test * use `Root` to allow specifying oss mode * Add fix and todo tests for reindexing with preMigrationScript * Dump execution log of state transitions and responses if we hit FATAL * add 7.3 xpack tests * add 100k test data * Reindex instead of cloning for migrations * Skip 100k x-pack integration test * MARK_VERSION_INDEX_READY_CONFLICT for dealing with different versions migrating in parallel * Track elapsed time * Fix tests * Model: make exhaustiveness checks more explicit * actions integration tests: add additional tests from CR * migrations_state_action_machine fix flaky test * Fix flaky integration test * Reserve FATAL termination only for situations which we never can recover from such as later version already migrated the index * Handle incompatible_mapping_exception caused by another instance * Cleanup logging * Fix/stabilize integration tests * Add REINDEX_SOURCE_TO_TARGET_VERIFY step * Strip tests archives of */.DS_Store and __MAC_OSX * Task manager migrations: remove invalid kibana property when converting legacy indices * Add disabled mappings for removed field in map saved object type * verifyReindex action: use count API * REINDEX_BLOCK_* to prevent lost deletes (needs tests) * Split out 100k docs integration test so that it has it's own kibana process * REINDEX_BLOCK_* action tests * REINDEX_BLOCK_* model tests * Include original error message when migration_state_machine throws * Address some CR nits * Fix TS errors * Fix bugs * Reindex then clone to prevent lost deletes * Fix tests Co-authored-by: Josh Dover <me@joshdover.com> Co-authored-by: pgayvallet <pierre.gayvallet@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # rfcs/text/0013_saved_object_migrations.md * createRootWithCorePlugins support for cliArgs which wasn't backported to 7.x * Attempt to stabilize cloneIndex integration tests (#86123) * Attempt to stabilize cloneIndex integration tests * Unskip test * return resolves/rejects and add assertions counts to each test * Await don't return expect promises * Await don't return expect promises for other tests too * Remove 8.0.0 integration test

rudolf requested a review from a team as a code owner September 24, 2020 12:27

rudolf marked this pull request as draft September 24, 2020 12:27

mshustov reviewed Sep 28, 2020

View reviewed changes

joshdover reviewed Oct 5, 2020

View reviewed changes

src/core/server/saved_objects/migrationsv2/index.ts Outdated Show resolved Hide resolved

rudolf force-pushed the so-migrations branch from 7d22ab2 to 3687918 Compare November 3, 2020 16:22

rudolf force-pushed the so-migrations branch 3 times, most recently from 67bcbe1 to 6ad7a54 Compare November 17, 2020 13:55

rudolf added project:ResilientSavedObjectMigrations Reduce Kibana upgrade failures by making saved object migrations more resilient Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Nov 27, 2020

rudolf mentioned this pull request Nov 27, 2020

Saved Object Migrations fail when a plugin is disabled #56731

Closed

rudolf force-pushed the so-migrations branch 2 times, most recently from 7d7db7c to 8d011b5 Compare December 2, 2020 16:15

rudolf added 14 commits December 4, 2020 09:56

Initial structure of migration state-action machine

4765c3e

Fix type import

1699561

Retries with exponential back off

915fdcd

Use discriminated union for state type

90cfda7

Either type for actions

e48d013

Test exponential retries

c630534

TaskEither types for actions

b85d986

Fetch indices instead of aliases so we can collect all index state in…

7f7e573

… one request

Log document id if transform fails

443a5d0

WIP: Legacy pre-migrations

4da5a84

UPDATE_TARGET_MAPPINGS

7911ee0

WIP OUTDATED_DOCUMENTS_TRANSFORM

8573d46

Narrow res types depending on control state

dca4c5b

OUTDATED_DOCUMENTS_TRANSFORM

7515240

rudolf requested a review from a team as a code owner December 14, 2020 11:52

pgayvallet reviewed Dec 14, 2020

View reviewed changes

rudolf added 10 commits December 15, 2020 04:25

REINDEX_BLOCK_* to prevent lost deletes (needs tests)

33cb185

Split out 100k docs integration test so that it has it's own kibana p…

342e597

…rocess

REINDEX_BLOCK_* action tests

80fed9c

REINDEX_BLOCK_* model tests

892ae1c

Include original error message when migration_state_machine throws

d0f2518

Address some CR nits

286bdfa

Fix TS errors

eea4f9e

Fix bugs

3c46888

Reindex then clone to prevent lost deletes

11dc7cf

Fix tests

e815df4

rudolf added v7.12.0 and removed v7.11.0 labels Dec 15, 2020

Merge branch 'master' into so-migrations

4a7f2f7

rudolf merged commit 89bd0fb into elastic:master Dec 15, 2020

rudolf deleted the so-migrations branch December 15, 2020 20:40

This was referenced Dec 17, 2020

v2 migrations needs to support re-writing document _id's to support saved-objects in multiple spaces #86247

Closed

[7.x] Resilient saved object migration algorithm (#78413) #86284

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 17, 2020

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 18, 2020

rudolf mentioned this pull request Feb 17, 2021

Remove v1 saved object migrations #91619

Closed

rudolf mentioned this pull request Mar 22, 2021

Implement SavedObject Migrations v2 #75780

Closed

17 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resilient saved object migration algorithm #78413

Resilient saved object migration algorithm #78413

rudolf commented Sep 24, 2020 •

edited

Loading

mshustov Sep 28, 2020

joshdover left a comment •

edited

Loading

kobelb commented Nov 30, 2020 •

edited

Loading

pgayvallet left a comment

pgayvallet Dec 14, 2020

pgayvallet Dec 14, 2020

pgayvallet Dec 14, 2020

pgayvallet Dec 14, 2020

pgayvallet Dec 14, 2020

rudolf Dec 15, 2020

pgayvallet Dec 14, 2020

rudolf commented Dec 15, 2020

rudolf commented Dec 15, 2020

kibanamachine commented Dec 17, 2020

kibanamachine commented Dec 18, 2020

kibanamachine commented Dec 18, 2020

Resilient saved object migration algorithm #78413

Resilient saved object migration algorithm #78413

Conversation

rudolf commented Sep 24, 2020 • edited Loading

Summary

Current status

Checklist

For maintainers

Choose a reason for hiding this comment

joshdover left a comment • edited Loading

Choose a reason for hiding this comment

kobelb commented Nov 30, 2020 • edited Loading

pgayvallet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rudolf commented Dec 15, 2020

rudolf commented Dec 15, 2020

kibanamachine commented Dec 17, 2020

kibanamachine commented Dec 18, 2020

💚 Build Succeeded

Metrics [docs]

Distributable file count

History

kibanamachine commented Dec 18, 2020

rudolf commented Sep 24, 2020 •

edited

Loading

joshdover left a comment •

edited

Loading

kobelb commented Nov 30, 2020 •

edited

Loading